feat(denario): onboard Denario — partner wallet, Gold/Silver Polygon assets, and permanent referral alias#4209
Conversation
|
Completed a full-diff conformance and logic review (1 round, 0 findings) before marking ready. Verified:
|
TaprootFreak
left a comment
There was a problem hiding this comment.
I found three blocking correctness/data-safety issues. The asset rows currently activate pay-in detection without providing a processable pricing/manual path, and both migrations can delete or overwrite state they did not create. Please address the inline findings before merge.
| ("name", "uniqueName", "type", "blockchain", "category", "dexName", "chainId", "decimals", "description", | ||
| "buyable", "sellable", "cardBuyable", "cardSellable", "instantBuyable", "instantSellable", | ||
| "paymentEnabled", "refEnabled", "refundEnabled", "ikna", "personalIbanEnabled", "comingSoon") | ||
| SELECT 'DGC', 'Polygon/DGC', 'Token', 'Polygon', 'Public', 'DGC', '0xf7e2d612f1a0ce09ce9fc6fc0b59c7fd5b75042f', 8, 'Denario Gold Coin', |
There was a problem hiding this comment.
[P1] Prevent inert DGC/DSC deposits from getting stuck in the pay-in pipeline. Supplying chainId makes Alchemy map inbound Polygon transfers to these assets because the register strategy loads all blockchain assets, regardless of isActive. The registration flow then calls validateInput(), which requires an asset→CHF price; with no priceRuleId, pricing throws and the catch leaves the pay-in in CREATED, so the minute job retries and logs it indefinitely. This contradicts the PR's claim that recognition/forwarding is harmless. Either provide the complete priced/manual processing path or explicitly exclude these inert assets from pay-in registration, and add a DGC/DSC deposit regression test.
| const value = JSON.stringify(refKeys); | ||
|
|
||
| if (row) { | ||
| await queryRunner.query(`UPDATE "setting" SET "value" = $1, "updated" = NOW() WHERE "key" = $2`, [ |
There was a problem hiding this comment.
[P1] Preserve and audit the previous ref-keys state before overwriting it. Production already has this setting, so this replaces the serialized value without a durable before→after record, contrary to CONTRIBUTING.md's critical “Auditable mutations — no destructive overwrites” rule. Rollback ownership is also unsafe: if denario already pointed to the same ref, up() returns without changing anything, but down() still deletes that pre-existing alias; it also deletes a value changed after deployment. Persist an immutable prior-state/audit record before the update (fail closed if it cannot be written) and make down() restore only the change actually owned by this migration.
| * @param {QueryRunner} queryRunner | ||
| */ | ||
| async down(queryRunner) { | ||
| await queryRunner.query(`DELETE FROM "asset" WHERE "uniqueName" IN ('Polygon/DGC', 'Polygon/DSC')`); |
There was a problem hiding this comment.
[P1] Do not delete rows that up() may not have created. Each up() insert is guarded by NOT EXISTS, but down() unconditionally deletes every matching asset and wallet (and wallet.name is not unique). In a pre-seeded environment or after migration-history recovery, an up→down cycle therefore removes pre-existing data. Either fail closed when matching rows already exist or persist exact migration ownership/prior state and revert only that state; add an up→down test covering the pre-existing-row case.
- pay-in register strategies recognize only priced assets (new AssetService.getPayInAssets); an unpriced token is no longer mapped into the register flow, where validateInput/pricing throws and would loop the pay-in in CREATED forever - AddDenarioPermanentRef: persist an immutable before-image of ref-keys before overwriting (fail closed in the migration transaction) and revert only the alias this migration set - AddDenarioWalletAndAssets: down() removes only still-inert assets and the wallet only while it has no owner or users - specs: unpriced-asset exclusion from recognition, ref-keys audit/ownership, wallet/asset up->down and ownership preservation
|
Thanks — addressed all three findings in 1. Inert DGC/DSC deposits stuck in the pay-in pipeline. Pay-in register strategies now recognize only priced assets via a new 2. 3. All local gates green (type-check, lint, 23 tests across the 3 new/updated specs). |
|
Follow-up review of
The referral controller change itself looks correct, and the DGC/DSC retry-loop cause is addressed. I reran the four targeted suites locally: 29 tests passed. The remaining blockers are test gaps and ownership/audit semantics, not failing CI. |
Overview
Bundles the full Denario onboarding into one PR:
Denarioto thewallettable (analogous to existing partner wallets, e.g. Cake Wallet).DGC,DSC) as inert, list-only assets.denarioalias on theref-keyssetting (original scope of this PR).All of it ships via a single reversible, idempotent migration each, plus the seed mirror for local dev.
1. Partner wallet
New migration
1784038000000-AddDenarioWalletAndAssets.jsinserts aDenariorow intowallet. Onlyname/displayNameare set; every compliance/behaviour column takes its conservative DB default (isKycClient=false,autoTradeApproval=false,usesDummyAddresses=false,displayFraudWarning=false,amlRules='0',buySpecificIbanEnabled=false) — kept conservative on purpose so the partner cannot bypass any check by default. Idempotent onname(not a unique column), reversible.2. Assets (Polygon ERC-20)
Added in the same migration and mirrored into
migration/seed/asset.csv(ids 411/412), values verified on-chain (decimals()/symbol()/name()):0xf7e2…042f0x5d4e…c7e7Contract addresses independently verified against Denario's official token pages (DGC
0xf7E2…042f, DSC0x5D4E…c7E7). Backing: 1 DGC = 1 oz gold, 1 DSC = 1 oz silver (999.9), vaulted in Switzerland.Both are inert, list-only (same pattern as
OlkyFrozen/EUR):type=Token,blockchain=Polygon,category=Public, nopriceRuleId, and every trade/payment flagfalse→isActive=false. Consequences:priceRuleId).financialTypeis intentionally leftnull(no precious-metal type exists yet) — set it when trading is enabled.Deliberately left empty (correct for an inert asset — not guessed)
financialType: no precious-metal type exists in the repo (values in use:Other/USD/EUR/BTC/CHF/DEPS/FPS, plus 2 empty). It drives fee-matching (fee.entity) and balance grouping (log-job) — introducing a new bucket is a tax/reporting classification decision for DFX/DAS, so leftnull.approxPriceUsd/Chf/Eur: would hard-code a decaying gold/silver snapshot; these fields are only read for active/custody/traded assets, so an inert asset never uses them.sortOrder: display-order only (e.g.dEURO/SANDalso leave it empty).Follow-ups when trading is enabled (verified inputs, need DFX/DAS decision)
DSC/USDCpool on Polygon (0x6722e2405a3b6ee5bf862112a143cf7228ee6c18, live ≈ $36.55 ≈ 1 oz silver). Denario also publishes Proof-of-Reserve oracles (DSC PoR0xb507a787cda02d6086f101d6067e9c35d58f1fc5). Attach aPriceRule(DEX/oracle source) then flip the buy/sell flags.financialTypeper token once the metal asset class is decided.isKycClient/amlRules).3. Referral alias (original scope)
denarioalias to the existingref-keyssetting/v1/app?code=<alias>to persist the resolved referral code instead of the unresolved alias, matching/v1/app/:appStable links after deployment
https://api.dfx.swiss/v1/app/services?code=denariohttps://dev.api.dfx.swiss/v1/app/services?code=denario/v1/app?code=denarioDeployment precondition
Each environment must contain exactly one active organization account whose trimmed, case-insensitive organization name is
DenarioorDenario AG, with exactly one active user that already has a valid referral code. The migration intentionally aborts if this invariant is not met. It also refuses to overwrite an existingdenarioalias that points elsewhere.Tests & checks
Wallet + assets migration
NOT EXISTSguards onuniqueName/name) and reversible (down()removes both assets + the wallet)Referral alias
pg-memto validate PostgreSQL semantics and reversible state changesnpm run type-check,npm run lint,npm run format:check,npm run build